-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Infer something for lambda without context #2634
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
(It doesn't fix anything, it only makes the hacks greppable)
@@ -480,7 +480,7 @@ def get_init_file(dir: str) -> Optional[str]: | |||
# These two are for backwards compatibility | |||
'silent_imports': bool, | |||
'almost_silent': bool, | |||
} | |||
} # type: Dict[str, Any] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Silence warning in line 562: "object" not callable
[case testInvalidContextForLambda] | ||
from typing import Callable | ||
f = lambda x: A() # type: Callable[[], A] | ||
f2 = lambda: A() # type: Callable[[A], A] | ||
class A: pass | ||
[out] | ||
main:2: error: Incompatible types in assignment (expression has type Callable[[Any], A], variable has type Callable[[], A]) | ||
main:2: error: Cannot infer type of lambda | ||
main:3: error: Incompatible types in assignment (expression has type Callable[[], A], variable has type Callable[[A], A]) | ||
main:3: error: Cannot infer type of lambda |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we don't need this error message now?
return AnyType() | ||
ret_type = self.accept(e.expr()) | ||
fallback = self.named_type('builtins.function') | ||
return callable_type(e, fallback, ret_type) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know how does the shadowing work, but it seems to work (lambda x: x
does not return the type of an outer x
)
With this PR in place, in PY2 mode, the following example (simplified from some real-world code) gives an error: def f(x):
return x.strip()
g = lambda line: map(f, line.split(':'))
lines = ['']
dict([g(line) for line in lines if len(g(line)) == 2]) # <-- here The error is:
|
I keep going back and forth over what's going on in that example. You'd think the type of But the type of def g(line: Any) -> XXX:
return map(f, line.split(':')) where g = lambda line: [line]
reveal_type(g('')) # E: Revealed type is 'builtins.list[Any]' Without this PR the type would just be So on the one hand I conclude that the original code (from which I boiled this down) would have a type error even if we had coerced the type of OTOH maybe we could do better by constructing a generic function from the lambda, so that the types here would correspond: g = lambda line: [line] # corresponds to Callable[[T], List[T]] |
Inferring generic lambda will be more precise, which will probably catch more errors; it won't silence anything that's not silenced now. So this suggestion is independent from the original example. One potential problem with inferring generic lambda is that it might be too strict - it will be hard to specify |
Inferring generic types for lambdas (when there's no type context) would require some heavy machinery (unless we special case some simple cases only). We only infer lambda argument types in trivial cases where the type context has a callable type, and I don't think that anything substantially more complicated is worth the effort right now. Instead of inferring |
Remember, the lambda syntax does not support type annotations. Brainstorming a little more, maybe in f = lambda x: ...
foo(f) we could just save the whole lambda as the value/type of |
(I'm also okay with merging this and maybe iterating later.) |
Hm, I found another regression in a different codebase. import re
p = re.compile(r".*")
print(p.sub(lambda *a, **k: "ho", "abc")) This was fine before, but now says
|
But the following fails with or without this PR: from typing import Callable
def f(t: Callable[[str], str]) -> str: ''
f(lambda *_: "") # E: Cannot infer type of lambda (Since It feels like it is unrelated to this PR, since this error happens before the decision to infer "something". I will try to find the cause anyway; maybe I'm wrong. |
(Sorry: of course this is related to this PR since there's no error without it) |
Ok, I get it: the error "occurs" without this PR too, but returning |
Maybe this case is the reason the original code had a TODO at the `return
AnyType()` suggesting to make it an error?
|
If so, I should move and rephrase the TODO. |
The real-world examples I found all basically just ignore any arguments
they receive. So it should be fine to infer Callable[..., X] where X is
determined by the expression only.
|
I think this specific case can be treated separately. For now there's a bandage on it. (I am also not sure about my previous conclusion that |
So I have one more real-world issue with this. Consider this example: from typing import Callable
def f(g = lambda *a, **k: None):
# type: (Callable[..., None]) -> None
g(1, 2) This gives an error:
The problem is that I don't even know how to silence this -- you can't put Thoughts? |
Turns out it's not about the parameters but about the return type. When you say There's actually a test for this behavior, which fixed #1425.
Yielding In other words, for this PR to work we need to decide whether I would expect And maybe it's not worth the trouble. |
Thanks for getting to the bottom of this. Your analysis sounds right, but I don't know what we should do about it either. Maybe @JukkaL has an opinion? I'll also ask @ddefisher. |
We think this will also break some things in our codebases; I'll report back with results in a little while. |
Actually it didn't break anything (and of course it fixed the example I gave). So I'm going to merge this. |
There may be a follow-up issue -- do we still need the "Cannot infer type of lambda" error? |
I have no idea. |
Hm, actually the issue in #2634 (comment) is still there, so I filed #2764 for that. |
Fix #2631